Skip to content

Plugin for custom evaluator for scaling metric evaluation #953

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

pchoudhury22
Copy link
Contributor

@pchoudhury22 pchoudhury22 commented Mar 10, 2025

Plugin for custom evaluator for scaling metric evaluation as discussed part of FLIP-514: Custom Evaluator plugin for Flink Autoscaler

What is the purpose of the change
Add interface for custom evaluator as plugin for Autoscaler.

Verifying this change
New unit tests added to cover this

Does this pull request potentially affect one of the following parts:
Dependencies (does it add or upgrade a dependency): no
The public API, i.e., is any changes to the CustomResourceDescriptors: no
Core observer or reconciler logic that is regularly executed: no

*/
Map<ScalingMetric, EvaluatedScalingMetric> evaluateVertexMetrics(
JobVertexID vertex,
Map<ScalingMetric, EvaluatedScalingMetric> evaluatedMetrics,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter seems redundant because the context already contains the evaluated Metrics map.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are calling the custom evaluator before adding the current vertex’s evaluated metrics to scaling output. and we have to pass the vertex as an argument, passing evaluatedMetrics separately to the evaluation context sounded good to me!

Let me know if it makes more sense to have the vertex's evaluated metrics as part of the evaluation context itself, will make that update! Thanks!

@@ -382,4 +385,19 @@ private static ConfigOptions.OptionBuilder autoScalerConfig(String key) {
"scaling.key-group.partitions.adjust.mode"))
.withDescription(
"How to adjust the parallelism of Source vertex or upstream shuffle is keyBy");

public static final ConfigOption<String> CUSTOM_EVALUATOR_NAME =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about deriving the name from the CustomEvaluator interface? We can load all the names when we load the custom evaluator implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for your inputs! That Sounds perfect! Will make the update!

…he interface. 2. Updating javadoc’s for the added methods.
@pchoudhury22 pchoudhury22 marked this pull request as ready for review April 29, 2025 07:46
@pchoudhury22 pchoudhury22 changed the title Draft PR for plugin based approach for custom evaluator for scaling metric evaluation Plugin for custom evaluator for scaling metric evaluation Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants